-
Notifications
You must be signed in to change notification settings - Fork 625
Fix nil reference in clienticon #4044
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix nil reference in clienticon #4044
Conversation
|
What's your use case @SPFabGerman ? The widget is meant to be instantiated, even in declarative structure. Just like It doesn't make sense to me to use the Maybe an evolution could be to allow declarative usage with the named parameters pattern. But that's really just a stylistic improvement. |
|
My use case is as a template for both a tasklist and a taglist: Because I use it in a template I can only specify the client with the two callbacks. But these seem to only be called after the widget has already been created. I know that I can use an The code has been adapted from https://awesomewm.org/apidoc/widgets/awful.widget.tasklist.html where pretty much the same thing is done. |
actionless
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i didn't had time to dig into the usecase, but i don't see any technical problem with this change
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4044 +/- ##
=======================================
Coverage 90.41% 90.42%
=======================================
Files 938 938
Lines 60232 60232
Branches 1139 1139
=======================================
+ Hits 54457 54463 +6
+ Misses 5269 5263 -6
Partials 506 506
🚀 New features to boost your workflow:
|
|
The awesome/lib/awful/widget/tasklist.lua Line 397 in 9475bb9
|
|
Oops, forget to approve with my comment. |
Swap condition order to check `obj._private.client == c` before accessing `.valid`, preventing crash when clienticon widget is created without a client (e.g., in declarative templates). Upstream: awesomeWM/awesome#4044
Swap condition order to check `obj._private.client == c` before accessing `.valid`, preventing crash when clienticon widget is created without a client (e.g., in declarative templates). Upstream: awesomeWM/awesome#4044
When a
clienticonwidget is created without specifying a client (for example when the widget is created declaratively), then theclientattribute is set tonil. But theproperty::iconsignal is called anyways, leading to an error because anilvalue is referenced in the first condition of theif-clause.I fixed this by swapping both conditions. Since the signal is only called for existing clients,
cis neverniland so the checkobj._private.client == cdoubles to ensure thatobj._private.clientexists and thatobj._private.client.validcan be referenced.